[RLlib] Quick-fix for default RLModules in combination with a user-provided config-sub-dict (instead of a full DefaultModelConfig).
#47965
Conversation
| # TODO (sven): Temporary fix (until we have figured out the final config | ||
| # architecture for default models). If self.model_config is a dict (which | ||
| # it should always be, make sure to merge it with the default config). | ||
| self.model_config = dataclasses.asdict(DefaultModelConfig()) | self.model_config |
There was a problem hiding this comment.
Does this mean that the user needs to do this everytime she defines a new module? We do here actually the same we did before with the AlgorithmConfig.model_config property.
There was a problem hiding this comment.
Does this mean that the user needs to do this everytime she defines a new module? We do here actually the same we did before with the
AlgorithmConfig.model_configproperty
Nope, this is only(!) needed for RLlib's default models. See my other answer here. Users are completely free to request any keys they need in their custom model classes and then - of course - have to provide these keys in the model_config dict they pass into the RLModuleSpec. Users can also decide to use default values inside their RLModule classes for certain keys.
| # TODO (sven): Temporary fix (until we have figured out the final config | ||
| # architecture for default models). If self.model_config is a dict (which | ||
| # it should always be, make sure to merge it with the default config). | ||
| self.model_config = dataclasses.asdict(DefaultModelConfig()) | self.model_config |
There was a problem hiding this comment.
I guess we need to add this also to the BCRLModule then.
There was a problem hiding this comment.
Also: We might need to add this to all the examples that are not derived from the algorithm modules.
There was a problem hiding this comment.
Great catch! Done ...
There was a problem hiding this comment.
For the non-default models, this is fine, b/c those are 100% user-driven and thus don't use the catalogs. The user decides, which keys are accessed in the model_config and only those need to be provided (or even have default values inside the custom RLModule code).
This is really only a problem for RLlib's default models.
sven1977
added
the
tests-ok
sven1977
added
rllib
…k_fix_default_model_config_problem
…ovided config-sub-dict (instead of a full `DefaultModelConfig`). (ray-project#47965) Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
…ovided config-sub-dict (instead of a full `DefaultModelConfig`). (ray-project#47965) Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
…ovided config-sub-dict (instead of a full `DefaultModelConfig`). (ray-project#47965) Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
…ovided config-sub-dict (instead of a full `DefaultModelConfig`). (ray-project#47965) Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
…ovided config-sub-dict (instead of a full `DefaultModelConfig`). (ray-project#47965) Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
…ovided config-sub-dict (instead of a full `DefaultModelConfig`). (ray-project#47965) Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
…ovided config-sub-dict (instead of a full `DefaultModelConfig`). (ray-project#47965) Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
…ovided config-sub-dict (instead of a full `DefaultModelConfig`). (ray-project#47965) Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
…ovided config-sub-dict (instead of a full `DefaultModelConfig`). (ray-project#47965) Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
Quick-fix for default RLModules in combination with a user-provided config-sub-dict (instead of a full
DefaultModelConfig).Why are these changes needed?
Related issue number
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/under thecorresponding
.rstfile.